✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Karthik-K-N · 2025-10-15T07:34:26Z

What this PR does / why we need it:

This PR adds EncryptionAlgorithm to KubeadmConfig and necessary changes

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes ##10077

/area provider/bootstrap-kubeadm

Karthik-K-N · 2025-10-15T07:36:22Z

Need to add Unit test cases and fix existing tests

Not super confident about the changes, Have broken some interfaces as well, Based on the initial review and if the path chosen is correct, I will update accordingly.

@sbueringer Please take a look when time permits.

sbueringer

Thx! I think I covered the open points, but just ping me if you want to discuss anything else.

Definitely feel free to ping me again after the next iteration on the prod code for another quick review

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

bootstrap/kubeadm/types/upstreamv1beta4/conversion.go

sbueringer · 2025-10-16T14:28:03Z

controlplane/kubeadm/internal/controllers/controller.go


 	// Generate Cluster Kubeconfig if needed
-	if result, err := r.reconcileKubeconfig(ctx, controlPlane); !result.IsZero() || err != nil {
+	if result, err := r.reconcileKubeconfig(ctx, controlPlane); err != nil || !result.IsZero() {


Please revert this one

Reverted it.

May not be either needed or relevant wrt to this PR, but I feel its better to maintain a standard for this, I see some places
!result.IsZero() || err != nil and in some other err != nil || !result.IsZero()
IDE suggesting to check error first

Got it. Makes sense and I agree with checking error first. Let's just do this everywhere and in a separate PR.

Feel free to open a PR, but please on top of #12857 / or after 12857 is merged (I get rid of some of these cases in that PR)

Sure, I will do it later. Thank you.

controllers/clustercache/cluster_accessor.go

util/certs/certs.go

util/certs/types.go

util/kubeconfig/kubeconfig.go

Karthik-K-N · 2025-10-17T07:53:39Z

@sbueringer thanks for the review, I have updated accordingly and ready for next round of review. Please take a look when time permints.

I tried in the approach of changing the existing thing rather than avoiding the breakage for users, I will remember it for the next time. Thanks

sbueringer · 2025-10-17T08:40:29Z

I tried in the approach of changing the existing thing rather than avoiding the breakage for users, I will remember it for the next time. Thanks

All good. I think it's a bit of a trade-off and probably also a case-by-case decision. Thx for asking for early feedback! I'll take another look

controllers/clustercache/cluster_accessor.go

controlplane/kubeadm/internal/workload_cluster.go

util/kubeconfig/kubeconfig.go

sbueringer · 2025-10-20T11:42:48Z

Did another quick round of review. I think the e2e test failure might just be a flake but let's see

/retest

If tests are green and the findings above are addressed I would do a full review (just need a bigger block of time for that)

util/kubeconfig/kubeconfig.go

controlplane/kubeadm/internal/control_plane.go

util/certs/certs.go

fabriziopandini · 2025-10-21T09:32:21Z

controlplane/kubeadm/internal/cluster_test.go

 			g.Expect(err).ToNot(HaveOccurred())

-			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey)
+			workloadCluster, err := m.GetWorkloadCluster(ctx, tt.clusterKey, "")


Let's use the default encryption algo instead of ""

rebase with main

Karthik-K-N · 2025-10-23T05:16:52Z

Some misconfiguration, will check
https://storage.googleapis.com/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_cluster-api/12859/pull-cluster-api-e2e-blocking-main/1981193968041332736/artifacts/clusters/bootstrap/logs/capi-system/capi-controller-manager/capi-controller-manager-7678cb554b-phkfl/manager.log

2025-10-23T03:20:49.644479347Z stderr F 	 stderr: error: error execution phase certs/apiserver: couldn't load CA certificate ca: failure loading ca certificate authority: failed to load key: couldn't load the private key file /etc/kubernetes/pki/ca.key: error reading private key file /etc/kubernetes/pki/ca.key: data does not contain a valid RSA or ECDSA private key

sbueringer · 2025-10-23T05:29:31Z

Maybe similar to #12859 (comment)

Karthik-K-N · 2025-10-23T08:02:20Z

Just checking again to make sure it passes.
/test pull-cluster-api-e2e-blocking-main

sbueringer · 2025-10-23T08:34:04Z

/test pull-cluster-api-e2e-main

Triggering all for more data

sbueringer · 2025-10-28T10:53:08Z

I'll try to review again ASAP. Hopefully tomorrow or so

sbueringer

@Karthik-K-N Thank you very much!

Last round of minor findings from my side

I also already did some testing with Tilt and everything looks good

util/certs/certs.go

util/kubeconfig/options.go

controlplane/kubeadm/internal/cluster.go

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

controlplane/kubeadm/internal/cluster.go

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

Karthik-K-N · 2025-10-31T06:55:53Z

Addressed all the review comments. Please take a look when time permits, Thanks.

util/conditions/:root:kubernetes:_output:local:go:bin:ginkgo -time

controlplane/kubeadm/internal/cluster.go

sbueringer · 2025-10-31T12:50:31Z

@Karthik-K-N Thx! Very last nit from my side.
You can also already open the PR for this one if you want: #12859 (comment) (but no rush)

/assign @fabriziopandini

sbueringer · 2025-10-31T12:52:19Z

/test pull-cluster-api-e2e-main

(please trigger ^^ again after you fixed the last finding)

k8s-ci-robot · 2025-10-31T14:44:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from fabriziopandini. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Karthik-K-N · 2025-10-31T14:47:05Z

/test pull-cluster-api-e2e-main

Karthik-K-N · 2025-10-31T14:47:30Z

@Karthik-K-N Thx! Very last nit from my side. You can also already open the PR for this one if you want: #12859 (comment) (but no rush)

/assign @fabriziopandini

I will submit a follow up soon, Thank you.

k8s-ci-robot · 2025-10-31T14:47:37Z

@Karthik-K-N: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-apidiff-main	`1c76d86`	link	false	`/test pull-cluster-api-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

sbueringer · 2025-10-31T15:29:31Z

Thx!

/lgtm

k8s-ci-robot · 2025-10-31T15:29:39Z

LGTM label has been added.

Git tree hash: 416ca4fa275bafe77248da3bcec1e942993edd94

k8s-ci-robot added area/provider/bootstrap-kubeadm Issues or PRs related to CAPBK cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 15, 2025

k8s-ci-robot requested review from elmiko and g-gaston October 15, 2025 07:34

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 15, 2025

sbueringer reviewed Oct 16, 2025

View reviewed changes

Karthik-K-N force-pushed the rsa-key branch from e29086e to 9b93039 Compare October 17, 2025 12:02

sbueringer reviewed Oct 20, 2025

View reviewed changes

util/kubeconfig/kubeconfig.go Outdated Show resolved Hide resolved

fabriziopandini reviewed Oct 21, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2025

Karthik-K-N added 6 commits October 23, 2025 08:13

Add EncryptionAlgorithm to Kubeadm's ClusterConfiguration

cd1a25d

Controller changes

41c4d39

Util changes

eefb585

Test changes

f8dac7e

Address review comments

418ff3d

Address review comments and rebase with main

f0820c0

rebase with main

Karthik-K-N force-pushed the rsa-key branch from 9ce5fd3 to f0820c0 Compare October 23, 2025 03:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 23, 2025

Address review comments

a41ee45

sbueringer reviewed Oct 30, 2025

View reviewed changes

sbueringer reviewed Oct 31, 2025

View reviewed changes

util/conditions/:root:kubernetes:_output:local:go:bin:ginkgo -time Outdated Show resolved Hide resolved

controlplane/kubeadm/internal/cluster.go Show resolved Hide resolved

k8s-ci-robot assigned fabriziopandini Oct 31, 2025

Address review comme

1c76d86

Karthik-K-N force-pushed the rsa-key branch from cb108b9 to 1c76d86 Compare October 31, 2025 14:44

k8s-ci-robot assigned sbueringer Oct 31, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 31, 2025

✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Are you sure you want to change the base?

✨ Add EncryptionAlgorithm to Kubeadmconfig #12859

Conversation

Karthik-K-N commented Oct 15, 2025

Uh oh!

Karthik-K-N commented Oct 15, 2025

Uh oh!

sbueringer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbueringer Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

sbueringer Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karthik-K-N commented Oct 17, 2025

Uh oh!

sbueringer commented Oct 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbueringer commented Oct 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fabriziopandini Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N commented Oct 23, 2025

Uh oh!

sbueringer commented Oct 23, 2025

Uh oh!

Karthik-K-N commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Oct 23, 2025

Uh oh!

sbueringer commented Oct 28, 2025

Uh oh!

sbueringer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karthik-K-N commented Oct 31, 2025

Uh oh!

Uh oh!

Uh oh!

sbueringer commented Oct 31, 2025

sbueringer left a comment •

edited

Loading

sbueringer Oct 17, 2025 •

edited

Loading

Karthik-K-N commented Oct 23, 2025 •

edited

Loading